Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes 13896: fix fullyQualifiedName field mapping for database schema search #13898

Merged
merged 3 commits into from
Nov 10, 2023
Merged

Fixes 13896: fix fullyQualifiedName field mapping for database schema search #13898

merged 3 commits into from
Nov 10, 2023

Conversation

mgorsk1
Copy link
Contributor

@mgorsk1 mgorsk1 commented Nov 8, 2023

Describe your changes:

Fixes 13896

I worked on elasticsearch mapping fix because current one breaks functionality of searching through schemas in database view.

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

@mgorsk1 mgorsk1 requested a review from a team as a code owner November 8, 2023 10:41
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 8, 2023

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@mgorsk1 mgorsk1 changed the title fix(search): fix fullyQualifiedName field mapping for database schema search Fixes 13896: fix fullyQualifiedName field mapping for database schema search Nov 8, 2023
@mgorsk1
Copy link
Contributor Author

mgorsk1 commented Nov 8, 2023 •
edited
Loading

more thorough analysis of elasticsearch index mappings shows that fullyQualifiedName is sometimes of text type, sometimes as keyword - unless this is a deliberate choice I would suggest refactoring to store always as keyword. I didn't want to do this in this PR as I don't know full scope of dependencies so I think this PR should be as minimal as possible to unblock the feature I've noticed broken by this.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 8, 2023

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@chirag-madlani chirag-madlani added the safe to test Add this label to run secure Github workflows on PRs label Nov 10, 2023
@sonarqubecloud SonarQubeCloud
Copy link

[open-metadata-ingestion] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@07Himank 07Himank merged commit 204cffd into open-metadata:main Nov 10, 2023
07Himank added a commit that referenced this pull request Nov 10, 2023
@mgorsk1 @07Himank
Fixes 13896: fix fullyQualifiedName field mapping for database schema…
ff08e52 
… search (#13898)

* fix fqn field mapping for database schema search

* 👌 Updating code due to code review changes.

---------

Co-authored-by: 07Himank <[email protected]>
ulixius9 pushed a commit that referenced this pull request Nov 10, 2023
@mgorsk1 @07Himank @ulixius9
Fixes 13896: fix fullyQualifiedName field mapping for database schema…
c137382 
… search (#13898)

* fix fqn field mapping for database schema search

* 👌 Updating code due to code review changes.

---------

Co-authored-by: 07Himank <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harshach harshach harshach left review comments

@07Himank 07Himank 07Himank approved these changes

Assignees
No one assigned
Labels
backend safe to test Add this label to run secure Github workflows on PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug: Filtering schema in database view doesn't work
4 participants
@mgorsk1 @harshach @07Himank @chirag-madlani